NO-JIRA: Fix flake in infracluster test - #578
openshift-merge-bot[bot] merged 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@mdbooth: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughUpdated test setup for the "Control Plane Machines but no ControlPlaneMachineSet" scenario to replace a fixed sleep delay with timestamp-based conditional waiting. The new logic ensures at least one second elapses since ChangesTest Timing Reliability
🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/controllers/infracluster/infracluster_controller_test.go (1)
174-175: Good fix for the race condition.The wait ensures the InfraCluster exists before attempting deletion at line 190, directly addressing the flake described in the PR.
Consider adding an explicit timeout for improved robustness and faster failure diagnosis:
// Wait for the InfraCluster to be created. Eventually(komega.Get(bareInfraCluster), "10s", "1s").Should(Succeed(), "InfraCluster should be created after machines exist")Based on learnings: "Review Ginkgo test code for... timeouts - include appropriate timeouts on Eventually/Consistently calls".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controllers/infracluster/infracluster_controller_test.go` around lines 174 - 175, Update the Eventually call that waits for the InfraCluster to include explicit timeout and poll interval parameters and a failure message: modify the Eventually(komega.Get(bareInfraCluster)).Should(Succeed()) usage to pass a sensible timeout and interval (e.g., "10s", "1s") and include a descriptive assertion message so the wait is bounded and failures are clearly reported when waiting for bareInfraCluster creation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/controllers/infracluster/infracluster_controller_test.go`:
- Around line 174-175: Update the Eventually call that waits for the
InfraCluster to include explicit timeout and poll interval parameters and a
failure message: modify the
Eventually(komega.Get(bareInfraCluster)).Should(Succeed()) usage to pass a
sensible timeout and interval (e.g., "10s", "1s") and include a descriptive
assertion message so the wait is bounded and failures are clearly reported when
waiting for bareInfraCluster creation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c36551a9-4ff5-41c5-b1a1-c90517bb8dd3
📒 Files selected for processing (1)
pkg/controllers/infracluster/infracluster_controller_test.go
|
/verified by CI |
openshift-ci-robot
added
the
verified
|
@mdbooth: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damdo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
This change only affects integration test code — no production code changes. Overriding all e2e tests. /override ci/prow/e2e-aws-capi-disconnected-techpreview ci/prow/e2e-aws-capi-techpreview ci/prow/e2e-aws-capi-techpreview-post-install ci/prow/e2e-aws-ovn ci/prow/e2e-aws-ovn-serial-1of2 ci/prow/e2e-aws-ovn-serial-2of2 ci/prow/e2e-aws-ovn-techpreview ci/prow/e2e-aws-ovn-techpreview-upgrade ci/prow/e2e-azure-capi-techpreview ci/prow/e2e-azure-ovn-techpreview ci/prow/e2e-azure-ovn-techpreview-upgrade ci/prow/e2e-gcp-capi-techpreview ci/prow/e2e-gcp-ovn-techpreview ci/prow/e2e-metal3-capi-techpreview ci/prow/e2e-openstack-capi-techpreview ci/prow/e2e-openstack-ovn-techpreview ci/prow/e2e-vsphere-capi-techpreview ci/prow/regression-clusterinfra-aws-ipi-techpreview-capi |
|
@mdbooth: Overrode contexts on behalf of mdbooth: ci/prow/e2e-aws-capi-disconnected-techpreview, ci/prow/e2e-aws-capi-techpreview, ci/prow/e2e-aws-capi-techpreview-post-install, ci/prow/e2e-aws-ovn, ci/prow/e2e-aws-ovn-serial-1of2, ci/prow/e2e-aws-ovn-serial-2of2, ci/prow/e2e-aws-ovn-techpreview, ci/prow/e2e-aws-ovn-techpreview-upgrade, ci/prow/e2e-azure-capi-techpreview, ci/prow/e2e-azure-ovn-techpreview, ci/prow/e2e-azure-ovn-techpreview-upgrade, ci/prow/e2e-gcp-capi-techpreview, ci/prow/e2e-gcp-ovn-techpreview, ci/prow/e2e-metal3-capi-techpreview, ci/prow/e2e-openstack-capi-techpreview, ci/prow/e2e-openstack-ovn-techpreview, ci/prow/e2e-vsphere-capi-techpreview, ci/prow/regression-clusterinfra-aws-ipi-techpreview-capi DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@mdbooth: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Fixes a timing error where we can try to delete the initially created
infracluster before it has been created.
Summary by CodeRabbit